Skip to content

[codex] Structure ACP schema generator errors#3362

Merged
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/acp-generator-errors
Jun 20, 2026
Merged

[codex] Structure ACP schema generator errors#3362
juliusmarminge merged 4 commits into
codex/redact-dpop-request-targetfrom
codex/acp-generator-errors

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • add structured Schema errors for ACP asset download/storage, document decoding, formatter processes, and generator invariants
  • preserve immediate HTTP, filesystem, schema, and process causes while attaching URL/path/command context
  • register downloaded-asset cleanup before either request so partial downloads are cleaned up on failure

Validation

  • vp test packages/effect-acp/scripts/generate.test.ts --no-cache
  • vp check (passes with pre-existing warnings)
  • vp run typecheck
  • vpr typecheck

Overlap audit

No current open PR touches either changed path, including previous filenames.


Note

Low Risk
Changes are confined to the dev-time schema generator script and its tests; success paths are unchanged aside from safer error shapes and download cleanup ordering.

Overview
Replaces the generic GenerateCommandError in the ACP schema generator with Effect Schema.TaggedError classes for download (HTTP vs filesystem), upstream JSON decode, bun oxfmt formatting, and malformed generated schema lines.

Download-related failures attach redacted URL context via getUrlDiagnostics from @t3tools/shared (length, protocol, hostname only—no full URL, credentials, or query in errors or messages). Underlying HTTP, FS, decode, and process failures are kept in cause fields with path, stage, and command metadata where useful.

downloadSchemas now registers the temp-file cleanup finalizer before starting downloads so partial failures still remove downloaded assets. Several pipeline steps are exported and covered by a new generate.test.ts suite. The CLI entrypoint is gated with import.meta.main so importing the script no longer runs the command.

Reviewed by Cursor Bugbot for commit a559447. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Structure ACP schema generator errors with tagged error types and safe URL diagnostics

  • Replaces generic thrown errors in generate.ts with typed TaggedError classes for each failure stage: HTTP download, filesystem write, document decode, formatter process/exit, and schema entry parsing.
  • Adds safe URL diagnostics (input length, protocol, hostname) to download errors so raw URLs are never exposed in error messages.
  • Extracts decodeUpstreamSchemaDocument, decodeMetaDocument, formatGeneratedFiles, and collectSchemaEntries into exported Effect functions with structured failure types.
  • Adds a if (import.meta.main) guard so importing the module no longer auto-runs the generate command.
  • Adds test coverage in generate.test.ts validating error fields, command spawning behavior, and schema parse failure modes.
  • Behavioral Change: collectSchemaEntries is now an Effect and callers must bind it; generateSchemas now propagates typed errors instead of GenerateCommandError.

Macroscope summarized a559447.

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: a623cb89-3cbc-457f-bf50-2b4319826e48

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/acp-generator-errors

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jun 20, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jun 20, 2026
@macroscopeapp

macroscopeapp Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

This PR refactors error handling in a code generation script by converting unstructured errors into typed Effect error classes. The changes are mechanical, don't alter the core logic flow, and include comprehensive tests for the new error structures.

You can customize Macroscope's approvability policy. Learn more.

juliusmarminge and others added 3 commits June 20, 2026 09:52
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
@juliusmarminge juliusmarminge force-pushed the codex/acp-generator-errors branch from 15152d6 to 162cbae Compare June 20, 2026 16:55
@juliusmarminge juliusmarminge changed the base branch from main to codex/redact-dpop-request-target June 20, 2026 16:55
@macroscopeapp macroscopeapp Bot dismissed their stale review June 20, 2026 16:55

Dismissing prior approval to re-evaluate 162cbae

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Schema errors thrown not yielded
    • Converted collectSchemaEntries from a plain function using throw to an Effect.fn generator using yield*, so AcpGeneratorSchemaValueDeclarationMissingError and AcpGeneratorSchemaNameParseError become typed failures instead of defects.

Create PR

Or push these changes by commenting:

@cursor push d506182388
Preview (d506182388)
diff --git a/packages/effect-acp/scripts/generate.ts b/packages/effect-acp/scripts/generate.ts
--- a/packages/effect-acp/scripts/generate.ts
+++ b/packages/effect-acp/scripts/generate.ts
@@ -313,9 +313,7 @@
   yield* fs.writeFileString(metaOutputPath, metaOutput);
 });
 
-function collectSchemaEntries(
-  chunk: string,
-): ReadonlyArray<{ readonly name: string; readonly code: string }> {
+const collectSchemaEntries = Effect.fn("collectSchemaEntries")(function* (chunk: string) {
   const lines = chunk
     .split("\n")
     .map((line) => line.trim())
@@ -330,14 +328,14 @@
 
     const constLine = lines[index + 1];
     if (!constLine?.startsWith("export const ")) {
-      throw new AcpGeneratorSchemaValueDeclarationMissingError({
+      return yield* new AcpGeneratorSchemaValueDeclarationMissingError({
         typeDeclaration: typeLine,
       });
     }
 
     const match = /^export type ([A-Za-z0-9_]+)/.exec(typeLine);
     if (!match?.[1]) {
-      throw new AcpGeneratorSchemaNameParseError({ typeDeclaration: typeLine });
+      return yield* new AcpGeneratorSchemaNameParseError({ typeDeclaration: typeLine });
     }
 
     entries.push({
@@ -348,7 +346,7 @@
   }
 
   return entries;
-}
+});
 
 function normalizeNullableTypes(value: Schema.Json): Schema.Json {
   if (Array.isArray(value)) {
@@ -431,7 +429,7 @@
 
   const output = generator.generate("openapi-3.1", normalizedDefinitions as never, false).trim();
   if (output.length > 0) {
-    for (const entry of collectSchemaEntries(output)) {
+    for (const entry of yield* collectSchemaEntries(output)) {
       if (!generatedEntries.has(entry.name)) {
         generatedEntries.set(entry.name, entry.code);
       }

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit 162cbae. Configure here.

Comment thread packages/effect-acp/scripts/generate.ts Outdated
Co-authored-by: codex <codex@users.noreply.github.com>
@juliusmarminge juliusmarminge merged commit d92c31b into codex/redact-dpop-request-target Jun 20, 2026
16 checks passed
@juliusmarminge juliusmarminge deleted the codex/acp-generator-errors branch June 20, 2026 18:34
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge added a commit that referenced this pull request Jun 21, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant